Skip to content

HYPERFLEET-1147 - feat: add caller identity support for audit attribution#120

Open
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1147-caller-identity
Open

HYPERFLEET-1147 - feat: add caller identity support for audit attribution#120
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1147-caller-identity

Conversation

@kuudori

@kuudori kuudori commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Inject caller identity headers into all E2E API requests so tests work against an API with identity enforcement enabled (production default).

  • Add IdentityConfig (header/value/token) to config with CLI flags, env vars (HYPERFLEET_IDENTITY_*), and config.yaml support
  • Inject identity via OpenAPI RequestEditorFn in helper.newHelper()
  • Add HaveAuditIdentity matcher and ExpectedIdentity() helper
  • Add created_by assertion in cluster creation, deleted_by in deletion
  • Deploy API with identity_header enabled in local kind setup
  • Update AGENTS.md, getting-started, and local-kind-setup docs

Summary

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

@openshift-ci openshift-ci Bot requested review from ciaranRoche and rh-amarin June 8, 2026 23:49
@openshift-ci

openshift-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sherine-k for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 03aa4d1c-3ef5-41d4-88fc-d2a6c8d85a15

📥 Commits

Reviewing files that changed from the base of the PR and between e286b25 and 5d8c6b8.

📒 Files selected for processing (14)
  • AGENTS.md
  • cmd/hyperfleet-e2e/main.go
  • cmd/hyperfleet-e2e/test/cmd.go
  • configs/config.yaml
  • deploy-scripts/lib/api.sh
  • docs/getting-started.md
  • docs/local-kind-setup.md
  • e2e/cluster/creation.go
  • e2e/cluster/delete.go
  • pkg/client/client.go
  • pkg/config/config.go
  • pkg/helper/helper.go
  • pkg/helper/matchers.go
  • pkg/helper/suite.go
✅ Files skipped from review due to trivial changes (3)
  • docs/local-kind-setup.md
  • docs/getting-started.md
  • AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (10)
  • cmd/hyperfleet-e2e/test/cmd.go
  • pkg/client/client.go
  • configs/config.yaml
  • deploy-scripts/lib/api.sh
  • pkg/helper/helper.go
  • e2e/cluster/delete.go
  • e2e/cluster/creation.go
  • pkg/helper/suite.go
  • pkg/helper/matchers.go
  • pkg/config/config.go

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features

    • Added caller identity configuration support with HTTP headers and bearer token authentication for API requests.
    • Implemented audit identity validation in E2E tests to verify request attribution and creation/deletion tracking.
  • Documentation

    • Updated getting started guide with required caller identity setup steps.
    • Expanded configuration documentation with identity header and environment variable references.

Walkthrough

This PR introduces caller identity configuration and audit attribution to the hyperfleet E2E test framework. Configuration schema defines identity header, value, and optional JWT token. The client wiring layer extends the OpenAPI client to accept a request editor that injects identity credentials into each outgoing HTTP request. Test helpers expose the configured identity and provide a Gomega matcher to validate audit fields on Cluster and Resource objects. E2E lifecycle tests for cluster creation and deletion now assert that audit fields match the configured caller identity, with assertions skipped when identity is not configured.

Sequence Diagram(s)

sequenceDiagram
  participant TestRunner
  participant Helper
  participant OpenAPIClient
  participant API
  TestRunner->>Helper: request client (uses config)
  Helper->>OpenAPIClient: create client with RequestEditor
  OpenAPIClient->>API: send HTTP request (adds X-HyperFleet-Identity / Authorization)
  API->>OpenAPIClient: respond (resource with audit CreatedBy/DeletedBy)
  OpenAPIClient->>TestRunner: return resource
  TestRunner->>Helper: assert HaveAuditIdentity(expected)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately summarizes the main feature: adding caller identity support for audit attribution across the E2E test suite.
Description check ✅ Passed Description is directly related to the changeset, covering configuration additions, OpenAPI injection, matchers, assertions, deployment changes, and documentation updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed Identity-related secrets properly redacted in logs via redactToken() function returning REDACTED, and token never logged in helper setup or deploy script.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/hyperfleet-e2e/main.go`:
- Around line 33-34: Add a new CLI flag binding for the identity token so
token-only usage is supported: declare or reuse the variable identityToken and
register it with pfs.StringVar(&identityToken, "identity-token", "", "Caller
identity token (alternative to identity-header/identity-value)"), and add the
same binding in the other flagset where identityHeader/identityValue are
registered (the other pfs.StringVar calls referenced in the comment). Update any
flag parsing logic that constructs request identity to prefer identityToken when
set (instead of header/value).

In `@cmd/hyperfleet-e2e/test/cmd.go`:
- Around line 60-67: The viper.BindPFlag calls for binding parent flags are
ignoring returned errors; update the block that calls viper.BindPFlag (for
config.API.URL, config.Log.Level, config.Log.Format, config.Log.Output,
config.Identity.Header, config.Identity.Value using parentFlags.Lookup("..."))
to check each error and handle it (e.g., return the error from the surrounding
function or log/fatal with context) instead of discarding; ensure you capture
the returned error from each viper.BindPFlag call and include a clear message
referencing the flag/key when reporting the failure.

In `@e2e/cluster/delete.go`:
- Around line 56-58: When h.ExpectedIdentity() returns a non-empty expected
identity, don't just assert deletedCluster.DeletedBy is non-nil — assert it
equals the expected identity for end-to-end attribution. Update the test in
delete.go to (1) fetch expected := h.ExpectedIdentity(), (2) ensure
deletedCluster.DeletedBy is present, and then (3) compare the DeletedBy value
against expected (use the appropriate field/representation on
deletedCluster.DeletedBy) so the assertion fails if the API attributes deletion
to the wrong caller.

In `@pkg/config/config.go`:
- Around line 497-499: The log currently emits c.Identity.Value as
"identity_value" in plaintext using valueOrNotSet; change this to a redacted
form (reuse or add a helper like redactValue similar to redactToken) so
identity_value does not reveal emails/identifiers in logs; update the call in
the block that builds the map (replace valueOrNotSet(c.Identity.Value) with a
redaction helper) and ensure redactToken(c.Identity.Token) remains unchanged.
- Around line 462-465: In Validate(), after the existing header/value presence
check (the block referencing c.Identity.Header and c.Identity.Value), add a
format validation for c.Identity.Value so malformed audit identities fail fast;
call or implement a small validator (e.g., parseAuditIdentity(...) or a tailored
regexp) and return a descriptive error from Validate() if it doesn't match the
expected audit-identity format (include the invalid value in the fmt.Errorf
message) so config loading fails early rather than at API runtime.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: f6bf6e8a-4652-469d-9a0d-4495a6cd0f70

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5ebcc and e286b25.

📒 Files selected for processing (14)
  • AGENTS.md
  • cmd/hyperfleet-e2e/main.go
  • cmd/hyperfleet-e2e/test/cmd.go
  • configs/config.yaml
  • deploy-scripts/lib/api.sh
  • docs/getting-started.md
  • docs/local-kind-setup.md
  • e2e/cluster/creation.go
  • e2e/cluster/delete.go
  • pkg/client/client.go
  • pkg/config/config.go
  • pkg/helper/helper.go
  • pkg/helper/matchers.go
  • pkg/helper/suite.go

Comment thread cmd/hyperfleet-e2e/main.go
Comment thread cmd/hyperfleet-e2e/test/cmd.go
Comment thread e2e/cluster/delete.go
Comment thread pkg/config/config.go
Comment thread pkg/config/config.go
…tion

Inject caller identity headers into all E2E API requests so tests work
against an API with identity enforcement enabled (production default).

- Add IdentityConfig (header/value/token) to config with CLI flags,
  env vars (HYPERFLEET_IDENTITY_*), and config.yaml support
- Inject identity via OpenAPI RequestEditorFn in helper.newHelper()
- Add HaveAuditIdentity matcher and ExpectedIdentity() helper
- Add created_by assertion in cluster creation, deleted_by in deletion
- Deploy API with identity_header enabled in local kind setup
- Update AGENTS.md, getting-started, and local-kind-setup docs
@kuudori kuudori force-pushed the HYPERFLEET-1147-caller-identity branch from e286b25 to 5d8c6b8 Compare June 9, 2026 00:00
@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown

@kuudori: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-deployment-validation 5d8c6b8 link true /test e2e-deployment-validation

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant